• X
• Mastodon
• Reddit
• LinkedIn

Approvers:

• myakove
• rnetser

Reviewers:

• myakove
• rnetser

• hold
• verified
• wip
• lgtm
• approve
• automerge

• Conventional Title: Mode: fix (claude/claude-opus-4-6-1m)
• Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6-1m)

• Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
• Committer Identity Check: Verifies last committer matches PR author
• Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

• Add DaemonSet.restart() to trigger rolling restarts via restartedAt template annotation.
• Add DaemonSet.wait_for_rollout() to wait for observed generation, updated, and available pods.
• Add unit tests covering restart patching and rollout completion/wait scenarios.

graph TD
  Caller["Consumer code"] --> DaemonSet["DaemonSet class"] --> K8sAPI{{"Kubernetes API"}}
  DaemonSet --> Sampler["TimeoutSampler"] --> K8sAPI
  K8sAPI --> Status["DaemonSet status"]
  Tests[/"Unit tests"/] --> DaemonSet
  Tests --> Sampler

  subgraph Legend
    direction LR
    _mod["Module"] ~~~ _ext{{"External"}} ~~~ _test[/"Tests"/]
  end

Loading

The following are alternative approaches to this PR:

1. Use DaemonSet status.conditions instead of counters

• ➕ Could align more closely with Kubernetes-native rollout signaling
• ➕ Potentially fewer numeric edge cases across Kubernetes versions
• ➖ Condition semantics can vary and are less deterministic for 'all pods updated' vs 'available' requirements
• ➖ May still require additional numeric checks for correctness

2. Watch-based rollout waiting (K8s watch/informer)

• ➕ Lower API polling overhead and faster reaction to changes
• ➕ More scalable for long rollouts
• ➖ More implementation complexity and error handling (reconnects, resourceVersion, timeouts)
• ➖ Harder to unit test than deterministic polling

3. Shell out to kubectl/oc rollout status

• ➕ Delegates rollout logic to mature CLI behavior
• ➕ Minimal code in library
• ➖ Adds runtime dependency on external binaries and cluster-specific tooling
• ➖ Harder to use in restricted environments and less portable in Python APIs

Recommendation: Current approach (restart via restartedAt annotation + polling on observedGeneration/updatedNumberScheduled/numberAvailable) is a pragmatic, portable solution that avoids CLI dependencies and fixes the known race in wait_until_deployed after restarts. If future issues arise across cluster versions, consider augmenting with condition checks, but keep numeric gates as the primary source of truth.

Enhancement (1) +56 / -0

daemonset.pyAdd DaemonSet restart() and rollout-aware wait_for_rollout() +56/-0

Add DaemonSet restart() and rollout-aware wait_for_rollout()

• Introduces restart() to trigger a rolling restart by patching the pod template with a kubectl.kubernetes.io/restartedAt timestamp. Adds wait_for_rollout() that polls the DaemonSet until the controller observed the latest generation and all desired pods are updated and available, avoiding the ready-count race in wait_until_deployed.

ocp_resources/daemonset.py

Tests (1) +135 / -0

test_daemonset.pyAdd unit tests for DaemonSet restart and rollout waiting +135/-0

Add unit tests for DaemonSet restart and rollout waiting

• Adds pytest-based unit tests validating restart() patch structure and wait_for_rollout() behavior across completion and several waiting scenarios by mocking TimeoutSampler iteration results.

tests/test_daemonset.py

Description

wait_for_rollout() assumes item.status and its fields are always present; if status is missing/None
(common early in reconciliation), the method can raise AttributeError/TypeError and abort waiting
instead of continuing until rollout completes.

Code

ocp_resources/daemonset.py[R86-94]

+            if sample.items:
+                item = sample.items[0]
+                status = item.status
+                desired_number_scheduled = status.desiredNumberScheduled
+                if (
+                    desired_number_scheduled > 0
+                    and status.observedGeneration == item.metadata.generation
+                    and status.updatedNumberScheduled == desired_number_scheduled
+                    and status.numberAvailable == desired_number_scheduled

Relevance

⭐⭐⭐ High

Team accepted polling hardening for missing/None status to avoid AttributeError (PR #2499).

PR-#2499

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new implementation dereferences item.status and status.desiredNumberScheduled without null
checks, which can throw if status is not populated. The repo previously fixed the same polling
anti-pattern by sampling self.exists and reading status via to_dict().get(...) to avoid
AttributeError when status is absent.

/ocp_resources/daemonset.py[85-96]
/ocp_resources/resource.py[1010-1025]
PR-#2499

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DaemonSet.wait_for_rollout()` directly dereferences `item.status` and several status fields without guarding for missing/None values. This can raise runtime exceptions during transient states (e.g., before the controller has populated `status`), causing the wait to fail early.

### Issue Context
The repository already uses a defensive pattern for waiting on status by sampling `self.exists` and extracting `status` via `to_dict()` + `.get(...)` to avoid attribute errors.

### Fix Focus Areas
- /ocp_resources/daemonset.py[76-96]
- /ocp_resources/resource.py[1010-1025]

### Suggested fix
- In `wait_for_rollout`, treat missing `status` or missing numeric fields as non-ready and `continue` polling rather than dereferencing.
- Prefer extracting values from `item.to_dict()` (or `sample.to_dict()` if you change the sampler to `func=lambda: self.exists`) and defaulting absent numeric fields to `0`.
- Add/extend unit tests to cover a sample where `status` is None/missing and ensure the loop continues until a valid complete state is observed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

wait_for_rollout() requires desiredNumberScheduled > 0 to return, so a DaemonSet with
desiredNumberScheduled==0 (e.g., no schedulable/matching nodes) will always time out even though
rollout is effectively complete.

Code

ocp_resources/daemonset.py[R89-95]

+                desired_number_scheduled = status.desiredNumberScheduled
+                if (
+                    desired_number_scheduled > 0
+                    and status.observedGeneration == item.metadata.generation
+                    and status.updatedNumberScheduled == desired_number_scheduled
+                    and status.numberAvailable == desired_number_scheduled
+                ):

Relevance

⭐⭐ Medium

No clear precedent on desired==0 rollout completion; existing patterns often gate on desired>0.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The DaemonSet implementation explicitly blocks completion for desired==0, while an analogous wait
helper (Deployment.wait_for_replicas) returns successfully in zero-desired/zero-actual scenarios to
avoid permanent timeouts.

/ocp_resources/daemonset.py[89-96]
/ocp_resources/deployment.py[375-385]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DaemonSet.wait_for_rollout()` hard-requires `desiredNumberScheduled > 0` before it can ever return. This makes the method hang until timeout for valid DaemonSets that currently have zero desired pods.

### Issue Context
Other rollout/wait helpers in this repo treat “desired == 0” as a valid terminal state and return successfully rather than timing out.

### Fix Focus Areas
- /ocp_resources/daemonset.py[89-96]
- /ocp_resources/deployment.py[375-385]

### Suggested fix
- Remove the `desired_number_scheduled > 0` guard, and instead allow completion when all compared values match even if they are 0.
- If you still want an explicit early-return case, treat `desiredNumberScheduled == 0` as complete once `observedGeneration == metadata.generation` (and optionally `updatedNumberScheduled == 0` and `numberAvailable == 0`).
- Add a unit test for `desired=0` to ensure `wait_for_rollout()` returns promptly rather than timing out.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new unit tests for wait_for_rollout() validate completion/waiting behavior based on
numberAvailable rather than the required numberReady condition. This means the test suite does
not cover the compliance-specified rollout-ready predicate and will not prevent regressions against
it.

Code

tests/test_daemonset.py[R104-113]

+    def test_wait_for_rollout_waits_when_not_all_available(self, daemonset):
+        not_available_response = _make_api_response(
+            generation=2,
+            observed_generation=2,
+            desired=3,
+            updated=3,
+            available=1,
+        )
+        not_available_response.items[0].status.numberReady = 3  # ready but not available yet
+

Relevance

⭐ Low

No precedent; tests align with PR’s numberAvailable intent, so likely kept as-is.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 4 requires tests to verify that wait_for_rollout() checks the specified rollout
conditions (including numberReady == desiredNumberScheduled). The added test
test_wait_for_rollout_waits_when_not_all_available explicitly sets numberReady to the desired
value but still expects waiting, demonstrating the tests are keyed to numberAvailable instead of
the required numberReady condition.

Automated tests added for DaemonSet.restart() and DaemonSet.wait_for_rollout()
DaemonSet provides a rollout-aware wait_for_rollout() method using generation and updated/ready scheduling checks
tests/test_daemonset.py[104-113]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `wait_for_rollout()` tests are written around `status.numberAvailable` semantics (including a case where `numberReady == desired` but the test expects to keep waiting). Compliance requires `wait_for_rollout()` to use `numberReady`.

## Issue Context
After updating `wait_for_rollout()` to check `numberReady`, the test cases should assert waiting/completion based on `numberReady` rather than `numberAvailable`.

## Fix Focus Areas
- tests/test_daemonset.py[13-25]
- tests/test_daemonset.py[44-135]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

DaemonSet.wait_for_rollout() uses status.numberAvailable == desiredNumberScheduled instead of
the required status.numberReady == desiredNumberScheduled, so it does not meet the specified
rollout-ready criteria. This is a direct mismatch with the compliance success criteria for the new
rollout-aware method.

Code

ocp_resources/daemonset.py[R89-95]

+                desired_number_scheduled = status.desiredNumberScheduled
+                if (
+                    desired_number_scheduled > 0
+                    and status.observedGeneration == item.metadata.generation
+                    and status.updatedNumberScheduled == desired_number_scheduled
+                    and status.numberAvailable == desired_number_scheduled
+                ):

Relevance

⭐ Low

No precedent; PR explicitly intends numberAvailable (minReadySeconds), so changing to numberReady
likely rejected.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2 requires status.numberReady == status.desiredNumberScheduled as a success
condition. The added code instead checks status.numberAvailable == desiredNumberScheduled, so it
does not implement the required readiness predicate.

DaemonSet provides a rollout-aware wait_for_rollout() method using generation and updated/ready scheduling checks
ocp_resources/daemonset.py[89-95]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DaemonSet.wait_for_rollout()` currently validates `status.numberAvailable == desiredNumberScheduled`, but the compliance requirement explicitly requires validating `status.numberReady == desiredNumberScheduled`.

## Issue Context
PR Compliance ID 2 defines the rollout completion criteria as: (1) `observedGeneration == generation`, (2) `updatedNumberScheduled == desiredNumberScheduled`, and (3) `numberReady == desiredNumberScheduled`.

## Fix Focus Areas
- ocp_resources/daemonset.py[89-95]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.